Skip to content

fix(cli): reload MCP config after project selection - #1179

Open
luantaraschi wants to merge 1 commit into
CodebuffAI:mainfrom
luantaraschi:fix/reload-agent-registry-on-project-change
Open

fix(cli): reload MCP config after project selection#1179
luantaraschi wants to merge 1 commit into
CodebuffAI:mainfrom
luantaraschi:fix/reload-agent-registry-on-project-change

Conversation

@luantaraschi

@luantaraschi luantaraschi commented Sep 1, 2026

Copy link
Copy Markdown

Recreated from #966, which GitHub auto-closed when this repository's history was rewritten. Same change, rebased onto the new main at d4902003. Fixes #957.

Picking a different project through the project picker changes the working directory, moves the project root and resets the client, but never re-scans .agents/ for the new directory. cachedAgentsDir and cachedAgentsByMode in local-agent-registry.ts go on serving the launch directory, so the selected project's MCP servers and local agents never reach the base agent definition.

handleProjectChange in cli/src/index.tsx had the chdir, setProjectRoot and resetCodebuffClient inline. This moves that sequence into activateProject() in project-picker.ts and puts the registry reload in the middle of it, so the ordering lives in one place instead of being duplicated at the call site.

reloadLocalAgentRegistry() clears only cachedAgentsDir and cachedAgentsByMode, and that is deliberate. The module keeps five mutable caches. The other three, userAgentsCache, userAgentFilePaths and mcpServersCache, are reassigned by initializeAgentRegistry() on both its success and failure paths, mcpServersCache from loadMCPConfigSync(), which reads the cwd. So getLoadedMCPServers() is covered as long as the chdir happens first. The two derived ones are the only ones nothing else resets.

The test writes real .agents/*.ts and mcp.json files into temporary launch and project directories, warms the caches in the launch project, switches through activateProject(), then asserts the selected project supplies the agent directory, the local agent list, the MCP server and the base agent definition, with the launch project's agent gone. A second case covers the { reloadAgentRegistry: false } skip path, where the chdir and project root still move but the launch project's agent is still the one served. I checked that case earns its place by making activateProject reload unconditionally: it is the only test that then fails.

Validation on this head:

bun test cli/src/__tests__/utils/     23 pass, 0 fail
cd cli && bun run typecheck           23 errors

The same command on the base, d4902003, also gives 23, so this change adds none. That command reaches past cli: 7 of them come from ../sdk/src/impl/llm.ts, 6 from ../common/src/util/messages.ts, and 10 from under cli/src. Of those 10, nine are react-dom/server declarations in component tests and the tenth is wrapper-safety.test.ts:265, a tar module that does not resolve. None of the four files this PR touches appears in the list.

An earlier revision of this description said 10 errors, all react-dom/server. That counted only the cli subset. Corrected here.

Picking a different project changed the working directory, moved the
project root and reset the client, but never re-scanned `.agents/` for
the new directory. `cachedAgentsDir` and `cachedAgentsByMode` in the
local agent registry kept serving the launch directory, so the selected
project's MCP servers and local agents never reached the base agent
definition.

Consolidates the chdir, project root, registry reload and client reset
into `activateProject()`, and clears the two cwd-derived caches before
`initializeAgentRegistry()` refreshes the rest.
@codebuff-team

Copy link
Copy Markdown
Contributor

Good bug fix. The root cause tracking is accurate: handleProjectChange in cli/src/index.tsx did the chdir/setProjectRoot/resetCodebuffClient dance but never invalidated cachedAgentsDir/cachedAgentsByMode in local-agent-registry.ts, so a project switch through the picker kept serving the launch directory's .agents/ and MCP config. Consolidating the sequence into activateProject() in project-picker.ts and adding reloadLocalAgentRegistry() is the right shape - it puts the ordering (chdir → setProjectRoot → reload → resetCodebuffClient) in one place instead of duplicated at the call site, and the reasoning about why only two of the five module caches need explicit clearing (the other three self-reassign on initializeAgentRegistry()) is sound and matches the code.

The test in project-picker.test.ts is a real integration test - it writes actual .agents/*.ts and mcp.json files to temp dirs, warms the launch project's cache, switches via activateProject(), and asserts the selected project's agent/MCP server/base-agent definition are what's served, with the launch agent gone. The reloadAgentRegistry: false case is a nice touch for the --agent override path and is shown to actually matter (fails if the reload becomes unconditional).

One thing to double check before porting: hasAgentOverride is added to the useCallback deps in index.tsx but isn't shown being declared in this diff - worth confirming it's stable/correctly scoped in the surrounding code, since a wrong dependency here would cause stale closures on the handler.

Small note: activateProject living in project-picker.ts while owning chdir/resetCodebuffClient blurs its original single responsibility (picker display logic) a little, but it's a reasonable trade-off for avoiding duplicated ordering logic, and the module doc comment on reloadLocalAgentRegistry explains the invariant well.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 1, 2026
@luantaraschi

Copy link
Copy Markdown
Author

It is stable, and checking it turned up something worth adding.

hasAgentOverride is declared at cli/src/index.tsx:221:

const hasAgentOverride = Boolean(agent?.trim())

That line sits inside main(), which opens at line 85, just below the parseArgs() destructuring that ends at 217. The component, AppWithAsyncAuth, does not start until line 304. So it is not render state or a prop, it is a closure capture from the scope around the component. parseArgs() reads argv once at startup, the binding is a const, and it is the only assignment to that name in the file. The value cannot differ between renders, so [hasAgentOverride] and [] behave the same and no stale closure is reachable.

Since you asked about the dependency itself: eslint.config.js does not load eslint-plugin-react-hooks, so exhaustive-deps is not enforced here. The entry documents the capture rather than satisfying a rule. Happy to drop it to [] if you would rather the array only hold things that can change.

While confirming that, I looked at the condition it feeds. reloadAgentRegistry: !hasAgentOverride mirrors the startup gate at line 261:

if (isPublishCommand || !hasAgentOverride) {
  await initializeAgentRegistry()
}

The isPublishCommand half cannot be reached from the picker. The publish block at 269 to 291 exits on both paths, process.exit(0) on success and process.exit(1) on failure, before the app renders, so a publish run never gets as far as the project picker. !hasAgentOverride is the same condition in the only state that reaches it.

One correction to my own description while you have this open. It says typecheck gives 10 errors, all react-dom/server declarations in component tests. Running the same command on this head and on its base d4902003 gives 23 both times. The delta is still zero, but the number is wrong: 7 come from ../sdk/src/impl/llm.ts, 6 from ../common/src/util/messages.ts, and 10 from under cli/src. Of those 10, nine are the react-dom/server declarations and the tenth is wrapper-safety.test.ts:265, a missing tar module in a release test. I counted only the errors under cli/src the first time and missed the ones coming from ../sdk and ../common.

bun test cli/src/__tests__/utils/ is 23 pass, 0 fail on this head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] MCP servers from .agents/mcp.json are not loaded when the project is selected via the project picker (CLI started from an ancestor directory)

2 participants